Update external postfit functionality#102
Conversation
…parameters after loading externalPostfit
…); Add possibility to only load parameters, not covariance from external postfit; always draw zero line in hist plot
lucalavezzo
left a comment
lucalavezzo
left a comment
There was a problem hiding this comment.
minor comments, I think good to merge if you want
| default=False, | ||
| action="store_true", | ||
| help="Do not profile beta parameters in the postfit (e.g. when using --externalPostfit).", | ||
| ) |
There was a problem hiding this comment.
It may be helpful to be more descriptive in this help section (or perhaps start a documentation?), because honestly I don't always follow the intricacies of these settings; in particular, here, I'm not sure what the use case for this is, and even a short sentence to give more context may help the layman user :)
There was a problem hiding this comment.
I've modified the help message. In general I agree that we should come up with a better documentation.
There was a problem hiding this comment.
So, is this option only about the bin-by-bin parameters ? In that case, what about choosing a name that directly targets that specific use case, e.g. --noPostfitProfileBB or something?
| h_den = h_inclusive | ||
|
|
||
| if diff: | ||
| h0 = hh.addHists(h_num, h_num, scale2=-1) |
There was a problem hiding this comment.
I guess you're using this to get an empty histogram with the same size as h_num, does this treat the variances correctly (if that matters)? Depending on the storage type, it may be summing the variances to find the final ones on h0.
There was a problem hiding this comment.
This is actually only used to draw the grey line around 1 (or 0 in case the difference is plot), without errors.
There was a problem hiding this comment.
But then isn't it more explicit to copy the histogram and set the content to 0 or 1?
There was a problem hiding this comment.
It would be the same
bin/rabbit_plot_pulls_and_impacts.py
Outdated
| ), | ||
| error_x=error_x, | ||
| name="Pulls ± Constraints", | ||
| name=f"Pulls ± Constraints ({name})", |
There was a problem hiding this comment.
if name is not passed?
davidwalter2
force-pushed
the
260126_externalPostfit
branch
from
080f580 to
79ff7ae
Compare
|
I'm also happy, the CI looks good. |
cippy
left a comment
cippy
left a comment
There was a problem hiding this comment.
Aside from the minor comments I made, which I leave it to you as you prefer, it is good to merge to me
| h_den = h_inclusive | ||
|
|
||
| if diff: | ||
| h0 = hh.addHists(h_num, h_num, scale2=-1) |
There was a problem hiding this comment.
But then isn't it more explicit to copy the histogram and set the content to 0 or 1?
| default=False, | ||
| action="store_true", | ||
| help="Do not profile beta parameters in the postfit (e.g. when using --externalPostfit).", | ||
| ) |
There was a problem hiding this comment.
So, is this option only about the bin-by-bin parameters ? In that case, what about choosing a name that directly targets that specific use case, e.g. --noPostfitProfileBB or something?
3 participants
The externalPostfit functionality is modified such that